-
Notifications
You must be signed in to change notification settings - Fork 228
Batched small updates #1003
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Batched small updates #1003
Conversation
| context. All reference to `x` within `E` are treated as having type `S` as | ||
| usual. | ||
| - Let `T` be the resulting inferred type of `E`. | ||
| - If `T` is assignable to `S` then no further work is required. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If T is a subtype of S which is of-interest, should there be a promotion here?
If T is dynamic, should there be a demotion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simplified this to just say that the usual promotion/demotion/error rules for an assignment of an expression of type T to a variable of current type S apply.
| type `T` to a variable of type `S`, that is: | ||
| - If the demotion policy applies, treat the assignment as a demoting | ||
| assignment which demotes `x` to `T`. | ||
| - Otherwise, it is an error, unless `T` is dynamic, in which case it is an |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dynamic is assignable to S, so it wouldn't reach here.
What type is the implicit downcast to? I'm assuming S.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, and yes.
| - If the demotion policy applies, treat the assignment as a demoting | ||
| assignment which demotes `x` to `T`. | ||
| - Otherwise, it is an error, unless `T` is dynamic, in which case it is an | ||
| implicit downcast. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it rather be:
- If
Tis a subtype ofS, then ifTis a type of interest forx,xis promoted toT,
and if it is not a type of interest, nothing further happens to the type ofx. (This includesTbeingS.)- If the demotion policy applies, treat the assignment as a demoting assignment which demotes
xtoT.- Otherwise if
Tisdynamic, then perform an implicit downcast fromTtoSon the value ofE.- Otherwise the assignment is a compile-time error.
(I assume that context-type based conversions like callable-object tear-off has happened during inference of E).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than re-specify all of the promotion/demotion policies, I've just changed this to say "at this point, treat it like any other assignment of something of type T to a variable of type S." I don't think we need any further text here do we? The point is just that we use the current type of x for downwards inference, but still consider the assignment as a potential promotion/demotion point.
resources/type-system/inference.md
Outdated
| Function literals which are inferred in an empty typing context (see below) are | ||
| inferred using the declared type for all of their parameters. If a parameter | ||
| has no declared type, it is treated as if it was declared with type `dynamic`. | ||
| has no declared type, it is treated as if it was declared with type `dynamic. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing a final ` after `dynamic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
|
Ok I addressed comments, landing this. I'll follow up if more comments come in on the revised text. cc @scheglov @johnniwinther for visibility. I believe all of the changes here are things we already clarified in issues and are most likely already implemented. I am adding some quick tests to verify. |
|
Tests out for review. |
Add a variety of small nnbd tests covering the specification changes/clarifications landed in dart-lang/language#1003 . Change-Id: I0716f14652128323bf103df154efb5bf978091d0 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/150160 Commit-Queue: Leaf Petersen <leafp@google.com> Reviewed-by: Bob Nystrom <rnystrom@google.com>
dart-lang/language#1003 Change-Id: Ie9526e8e362865754e84b5d9cda307dc7b07aaee Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/150162 Reviewed-by: Brian Wilkerson <brianwilkerson@google.com> Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
|
|
||
| ### Typedefs defined in legacy libraries used in opted-in libraries | ||
|
|
||
| A typedef which is define in a legacy library and used in an opted-in library is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
defined
Several small updates to questions and clarifications that have been resolved in the issue tracker but not moved to the spec.